Jdk25 #111
Jdk25 #111
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughCentralizes jlink option handling in Alpine and Debian Dockerfiles, adds explicit JDK 25 branching with a selective module set, and updates docker-bake.hcl to include JDK 25, a JAVA25_VERSION variable, and a Windows-specific JDK build matrix variable. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Bake as docker-bake.hcl
participant BK as BuildKit
participant DF as Dockerfile (alpine/debian)
participant JL as jlink
Dev->>BK: bake build
BK->>Bake: load variables & targets
Note over Bake: jdks_to_build=[17,21,25]\njdks_to_build_windows=[17,21]\nJAVA25_VERSION present
Bake-->>BK: target matrix (OS × JDK)
loop For each target
BK->>DF: build with JAVA_VERSION
activate DF
DF->>DF: assemble jlink options via case → populate "$@"
alt JAVA_VERSION starts with 17
DF->>JL: jlink "$@" --compress=2 + --add-modules ALL-MODULE-PATH + common opts
else JAVA_VERSION starts with 21
DF->>JL: jlink "$@" --compress=zip-6 + --add-modules ALL-MODULE-PATH + common opts
else JAVA_VERSION starts with 25
DF->>JL: jlink "$@" --compress=zip-6 + --add-modules java.base,java.logging,java.xml,java.management,java.net.http,jdk.crypto.ec + common opts
else Unsupported
DF-->>BK: error (unsupported JAVA_VERSION)
end
JL-->>DF: produce /javaruntime
DF-->>BK: image layer with runtime
deactivate DF
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @gounthar, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request integrates JDK 25 into the Docker build process, enabling the creation of container images with the latest Java development kit. It involves updating Dockerfiles to handle JDK 25's jlink requirements for optimized runtime images and modifying the docker-bake.hcl configuration to include JDK 25 in the build matrix and refine platform-specific build targets.
Highlights
- JDK 25 Support: Introduced support for building Docker images with JDK 25 across both Alpine and Debian base images.
- Jlink Configuration for JDK 25: The
jlinkcommand for JDK 25 now explicitly specifies a subset of modules (java.base,java.logging,java.xml,java.se) instead ofALL-MODULE-PATH, aiming for smaller runtime images. Commonjlinkoptions were also refactored into a variable. - Build Configuration Updates: The
docker-bake.hclfile has been updated to include JDK 25 in the default build matrix for non-Windows platforms and defines its version. A new variablejdks_to_build_windowswas added to manage JDK versions specifically for Windows builds. - Platform Targeting Refinement: The
alpine_platformsfunction was adjusted to enablelinux/arm64builds for JDK 21 and JDK 25, while JDK 17 remainslinux/amd64only. Windows targets (nanoserverandwindowsservercore) now exclusively use thejdks_to_build_windowsvariable, limiting them to JDK 17 and 21.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request adds support for JDK 25 to the Docker images. The changes correctly update the build configuration and Dockerfiles. My review includes suggestions to simplify the module list for the JDK 25 jlink command and raises a concern about a potential breaking change in the module inclusion strategy for JDK 25, which deviates from the approach used for previous JDK versions.
…rity and maintainability
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docker-bake.hcl (2)
38-40: Windows matrix excludes 25 by default—intentional?Parity note: Linux builds include 25; Windows does not. If intentional pending GA, consider a comment in-file or env flag to enable later.
101-106: Make the javaversion mapping explicit for 25.Current fallback maps any non-17/21 to JAVA25_VERSION. Being explicit avoids silent misbuilds if someone passes 19/22.
Apply this diff:
- result = (equal(17, jdk) - ? "${JAVA17_VERSION}" - : equal(21, jdk) - ? "${JAVA21_VERSION}" - : "${JAVA25_VERSION}") + result = (equal(17, jdk) ? "${JAVA17_VERSION}" + : (equal(21, jdk) ? "${JAVA21_VERSION}" + : (equal(25, jdk) ? "${JAVA25_VERSION}" : "${JAVA25_VERSION}")))debian/Dockerfile (1)
49-60: Version pattern robustness.Relying on jlink --version string shape is brittle. Prefer parsing $JAVA_VERSION’s major instead.
Optional diff:
- case "$(jlink --version 2>&1)" in \ - "17."*) jlink $JLINK_COMMON_OPTS \ + case "${JAVA_VERSION}" in \ + 17.*) jlink $JLINK_COMMON_OPTS \ --compress=2 \ --add-modules ALL-MODULE-PATH ;; \ - "21."*) jlink $JLINK_COMMON_OPTS \ + 21.*) jlink $JLINK_COMMON_OPTS \ --compress=zip-6 \ --add-modules ALL-MODULE-PATH ;; \ - "25"*) jlink $JLINK_COMMON_OPTS \ + 25*) jlink $JLINK_COMMON_OPTS \ --compress=zip-6 \ - --add-modules java.base,java.logging,java.xml,java.se ;; \ + --add-modules java.base,java.logging,java.xml ;; \ *) echo "ERROR: unmanaged jlink version pattern" && exit 1 ;; \ esac
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
alpine/Dockerfile(1 hunks)debian/Dockerfile(1 hunks)docker-bake.hcl(5 hunks)
🔇 Additional comments (6)
docker-bake.hcl (4)
35-36: Confirm adding 25 to the default Linux matrix (blast radius).Including 25 by default will publish new image tags. Please confirm CI, downstream consumers, and release notes are ready.
78-81: Verify JAVA25_VERSION format works with jdk-download scripts.The value "25+9-ea-beta" may not match expected patterns. Please confirm download/resolution succeeds across Linux/Windows.
112-115: Alpine platforms: 17 only on amd64—double-check requirement.21/25 build for amd64+arm64; 17 is amd64-only. If 17 arm64 was previously produced or expected, update here.
204-205: Switch to jdks_to_build_windows looks good.Matrix centralization improves maintainability; no functional concerns.
Also applies to: 229-230
debian/Dockerfile (1)
48-60: Remove java.se from jlink 25. — it pulls most SE modules and bloats the runtime.*
Replace java.se with an explicit minimal module list; apply diff:- "25"*) jlink $JLINK_COMMON_OPTS \ - --compress=zip-6 \ - --add-modules java.base,java.logging,java.xml,java.se ;; \ + "25"*) jlink $JLINK_COMMON_OPTS \ + --compress=zip-6 \ + --add-modules java.base,java.logging,java.xml ;; \Validate these modules for Jenkins agent workloads — JNDI/Unsafe may require java.naming and jdk.unsupported.
alpine/Dockerfile (1)
46-58: Mirror Debian fixes: use build ARG for jlink selection and drop java.se for JDK 25Avoid parsing
jlink --versionoutput (brittle) and removejava.se(it expands the runtime). Files: alpine/Dockerfile, debian/DockerfileApply this diff:
- case "$(jlink --version 2>&1)" in \ - "17."*) jlink $JLINK_COMMON_OPTS \ + case "${JAVA_VERSION}" in \ + 17.*) jlink $JLINK_COMMON_OPTS \ --compress=2 \ --add-modules ALL-MODULE-PATH ;; \ - "21."*) jlink $JLINK_COMMON_OPTS \ + 21.*) jlink $JLINK_COMMON_OPTS \ --compress=zip-6 \ --add-modules ALL-MODULE-PATH ;; \ - "25"*) jlink $JLINK_COMMON_OPTS \ + 25*) jlink $JLINK_COMMON_OPTS \ --compress=zip-6 \ - --add-modules java.base,java.logging,java.xml,java.se ;; \ + --add-modules java.base,java.logging,java.xml ;; \ *) echo "ERROR: unmanaged jlink version pattern" && exit 1 ;; \ esacValidate that java.base,java.logging,java.xml are sufficient for Jenkins agent usage.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
debian/Dockerfile (1)
51-51: Reduce to aggregator modulejava.se.
java.base,java.logging,java.xmlare redundant whenjava.seis present. Use onlyjava.se.Apply this diff:
- "25"*) set -- "--compress=zip-6" --add-modules java.base,java.logging,java.xml,java.se ;; \ + "25"*) set -- "--compress=zip-6" --add-modules java.se ;; \alpine/Dockerfile (1)
49-49: Reduce to aggregator modulejava.se.Same redundancy:
java.sealready includesjava.base,java.logging, andjava.xml.Apply this diff:
- "25"*) set -- "--compress=zip-6" --add-modules java.base,java.logging,java.xml,java.se ;; \ + "25"*) set -- "--compress=zip-6" --add-modules java.se ;; \
🧹 Nitpick comments (2)
debian/Dockerfile (1)
49-53: Intentional change? 25.x switches from ALL-MODULE-PATH to a limited set.This diverges from the comment above (“include the full module path to maintain compatibility”) and may break workloads expecting additional
jdk.*tools. Please either document the change explicitly or gate it behind a build arg with a safe default.Minimal toggle (defaults to compatibility):
- "25"*) set -- "--compress=zip-6" --add-modules java.se ;; \ + "25"*) set -- "--compress=zip-6" --add-modules "${JLINK_ADD_MODULES:-ALL-MODULE-PATH}" ;; \And add (outside this hunk) near the other ARGs:
ARG JLINK_ADD_MODULES # Example: --build-arg JLINK_ADD_MODULES=java.se for a smaller JREalpine/Dockerfile (1)
47-51: Avoid surprise breakage: make the 25.x module reduction opt-in.To preserve backward compatibility, default 25.x to ALL-MODULE-PATH and allow opting into
java.sevia a build arg.- "25"*) set -- "--compress=zip-6" --add-modules java.se ;; \ + "25"*) set -- "--compress=zip-6" --add-modules "${JLINK_ADD_MODULES:-ALL-MODULE-PATH}" ;; \Add (outside this hunk), alongside other ARGs:
ARG JLINK_ADD_MODULES
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
alpine/Dockerfile(1 hunks)debian/Dockerfile(1 hunks)
🔇 Additional comments (2)
debian/Dockerfile (1)
49-51: Confirm--compress=zip-6support across JDK vendors.Some distributions may lag feature flags. Please confirm this flag works with all vendors you build (e.g., Temurin) for JDK 21/25.
alpine/Dockerfile (1)
47-49: Verify--compress=zip-6on Alpine’s JDK.Please confirm this flag is supported by the Alpine JDK you download (some builds may differ).
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
debian/Dockerfile (1)
49-53: JDK 25 switches to a minimal module set — document and make overridable.This is a breaking change versus 17/21’s ALL-MODULE-PATH and contradicts the nearby compatibility comment. Please:
- Document the intent in-file/PR.
- Allow override via build-arg so users can quickly fall back to broader sets if something breaks.
Apply within this hunk:
- "25"*) set -- "--compress=zip-6" --add-modules java.base,java.logging,java.xml,java.management,java.net.http,jdk.crypto.ec ;; \ + "25"*) mod_list="${JLINK_MODULES:-java.base,java.logging,java.xml,java.management,java.net.http,jdk.crypto.ec}"; \ + set -- "--compress=zip-6" --add-modules "${mod_list}" ;; \And add (once, near other ARGs in this stage):
ARG JLINK_MODULESVerification (parity and easy diff between Debian/Alpine):
#!/bin/bash rg -nP '25"\*\).*--add-modules\s+([^\s;]+)' debian/Dockerfile alpine/Dockerfile -C1alpine/Dockerfile (1)
47-51: Same JDK 25 minimal module change — mirror Debian fixes and make it overridable.Replicate the build-arg override and add brief rationale to avoid surprises for users upgrading to 25.
Apply within this hunk:
- "25"*) set -- "--compress=zip-6" --add-modules java.base,java.logging,java.xml,java.management,java.net.http,jdk.crypto.ec ;; \ + "25"*) mod_list="${JLINK_MODULES:-java.base,java.logging,java.xml,java.management,java.net.http,jdk.crypto.ec}"; \ + set -- "--compress=zip-6" --add-modules "${mod_list}" ;; \And add (once, near other ARGs in this stage):
ARG JLINK_MODULES
🧹 Nitpick comments (4)
debian/Dockerfile (2)
49-51: Version pattern and option support — minor consistency check.
- Pattern "25") will match both “25” and “25.0.x”; good. Consider aligning 17/21 to 17")/21"*) for uniformity, or keep as-is.
- Please confirm jlink on your 21/25 vendors supports --compress=zip-6.
54-59: Improve build traceability of jlink options.Echo the computed args to help triage failures and confirm the module set in logs.
jlink \ --strip-java-debug-attributes \ - "$@" \ + $(echo "$@" | sed 's/^/ /') \ + && echo "jlink args: $@" \ --no-man-pages \ --no-header-files \ --output /javaruntimealpine/Dockerfile (2)
46-50: Minor: version match consistency and zip-6 support.Same notes as Debian: pattern “25"*) is fine; please verify --compress=zip-6 availability for your 21/25 builds.
52-57: Log chosen jlink options for Alpine as well.Echo computed args to aid debugging and reproducibility.
jlink \ --strip-java-debug-attributes \ - "$@" \ + $(echo "$@" | sed 's/^/ /') \ + && echo "jlink args: $@" \ --no-man-pages \ --no-header-files \ --output /javaruntime
Summary by CodeRabbit
New Features
Refactor